Conversation
|
@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration. |
|
Claude finished @arthanson's task in 2m 27s —— View job PR Review: #20897 - Expose selection custom field labels in the REST API
Two issues of potential concern: 1. Breaking API change for existing integrations SELECT and MULTISELECT custom fields previously returned raw strings (e.g. This is the same tradeoff made when similar changes were applied to built-in choice fields, so it may be intentional, but it should be called out explicitly in the PR as a breaking change. 2. Potential 500 error when a client writes back the read format for MULTISELECT
if not set(value).issubset(self.choice_set.values):
The SELECT case produces a legible A minimal fix in |
|
pre classed review 1 is known it's a breaking change. 2 fixed - now consistent across the board: read returns {value, label}; write takes the raw value; the object form is rejected with a 400 for both selection types - same as built-in choice fields. |
| ```json | ||
| "custom_fields": { | ||
| "cluster": { | ||
| "value": 1, |
There was a problem hiding this comment.
This reads like a PK integer, but I think this would actually be a database stored string, yeah?
| } | ||
| ``` | ||
|
|
||
| As with built-in choice fields, selection custom fields are written by passing the raw value (e.g. `"cluster": 1`), not the `{value, label}` object returned on read. |
There was a problem hiding this comment.
As with built-in choice fields, selection custom fields are written by passing the raw value (e.g. "site_type": "datacenter"), not the {value, label} object returned on read, right?
Closes: #20897
Selection and multiple-selection custom fields are now rendered in the REST API as objects exposing both the stored value and its resolved label ({"value": …, "label": …}), matching the convention already used by built-in choice fields. The change affects read access only; values are still written by passing the raw value.